Skip to content

fix(security): remove shell=True from generator drift runner (LED-713)#3

Merged
infracore merged 1 commit intomainfrom
fix/generator-drift-injection
Apr 7, 2026
Merged

fix(security): remove shell=True from generator drift runner (LED-713)#3
infracore merged 1 commit intomainfrom
fix/generator-drift-injection

Conversation

@infracore
Copy link
Copy Markdown
Member

Summary

  • `delimit_security_audit` flagged `core/generator_drift.py:104` for `subprocess(shell=True)` — a command injection surface since the `generator_command` input flows from caller workflow files straight to a shell
  • Fixed with `shlex.split()` + `shell=False` and explicit shell metacharacter rejection
  • Added `tests/test_generator_drift.py` with 17 tests covering injection guards, happy path, error handling, and report formatting

Why this matters

This is the first gate-driven fix in the v1.9.0 re-release. The previous attempt at v1.9.0 was rolled back because the deploy gates (`security_audit`, `test_smoke`, `changelog`, `deploy_plan`) were skipped. Running them properly caught this bug before it shipped.

Test plan

  • 17 new tests in `tests/test_generator_drift.py` — all passing
  • 88 existing tests still passing (105 total)
  • Real agentspec `pnpm run schema:export` still works (8.1s)
  • `delimit_security_audit` now clean — 0 findings (was 1 medium)

What changed in generator_drift.py

Before:
```python
result = subprocess.run(
regen_command,
shell=True, # ← command injection surface
...
)
```

After:
```python
argv = shlex.split(regen_command)

Reject shell metacharacters — force script files for chaining

if any(ch in token for token in argv for ch in "&|;><`$"):
return DriftResult(error="generator_command contains shell metacharacters...")
result = subprocess.run(argv, shell=False, ...)
```

Users needing shell features (chaining, env vars, redirection) should point `generator_command` at a script file instead of inline-chaining — documented in the rejection error message.

🤖 Generated with Claude Code

delimit_security_audit flagged core/generator_drift.py:104 for
subprocess(shell=True) — a real command injection surface, since the
generator_command input flows from the caller's workflow file straight
to a shell.

Fix:
- Parse regen_command with shlex.split and run with shell=False
- Reject shell metacharacters (&, |, ;, >, <, `, $) with a clear error
  message pointing users at script files for chaining/env vars/etc.
- Handle FileNotFoundError explicitly for missing executables
- Handle shlex.split ValueError for unparseable commands
- Reject empty commands

Also adds tests/test_generator_drift.py (17 tests):
- 8 injection-guard tests (chain, pipe, redirect, backtick, $(), ;, empty, unparseable)
- 3 happy-path tests (simple command, drift detection, workspace restore)
- 4 error-handling tests (missing artifact, missing executable, exit code, timeout)
- 2 formatter tests

Verified:
- All 88 prior tests still passing, 105 total (88 existing + 17 new)
- Real agentspec 'pnpm run schema:export' still works (8.1s)
- delimit_security_audit now clean (0 findings)

This fix makes the v1.9.0 release candidate safe to ship. The
previously-rolled-back release bundled the same code with the shell=True
bug.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@infracore infracore merged commit 8570365 into main Apr 7, 2026
3 of 5 checks passed
@infracore infracore deleted the fix/generator-drift-injection branch April 7, 2026 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant